Skip to content

treewide: introduce formatter#314

Merged
tpwrules merged 3 commits into
mainfrom
unknown repository
Aug 2, 2025
Merged

treewide: introduce formatter#314
tpwrules merged 3 commits into
mainfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Jun 29, 2025

Copy link
Copy Markdown

Since this is under the nix-community banner, this project should decide on a formatter. This PR currently chooses nixfmt-rfc-style via github:numtide/treefmt-nix flake, formats all files, and adds a .git-blame-ignore-revs file to ignore this mass reformat when blaming.

This would also allow for pre-commit hooks for formatting all files as well as automatic merge checks when opening additional PRs which reduces the chance of merge conflicts occurring.

@ghost

ghost commented Jul 4, 2025

Copy link
Copy Markdown
Author

Resolved merge conflicts and rebased on main.

@ghost

ghost commented Jul 4, 2025

Copy link
Copy Markdown
Author

This is what the deadnix formatter also includes when it's run.

diff --git a/apple-silicon-support/packages/alsa-ucm-conf-asahi/default.nix b/apple-silicon-support/packages/alsa-ucm-conf-asahi/default.nix
index 6e1f324..413fffd 100644
--- a/apple-silicon-support/packages/alsa-ucm-conf-asahi/default.nix
+++ b/apple-silicon-support/packages/alsa-ucm-conf-asahi/default.nix
@@ -1,5 +1,4 @@
 {
-  lib,
   fetchFromGitHub,
   alsa-ucm-conf,
 }:
diff --git a/apple-silicon-support/packages/asahi-audio/default.nix b/apple-silicon-support/packages/asahi-audio/default.nix
index 7c0582c..553c739 100644
--- a/apple-silicon-support/packages/asahi-audio/default.nix
+++ b/apple-silicon-support/packages/asahi-audio/default.nix
@@ -1,6 +1,5 @@
 {
   stdenv,
-  lib,
   fetchFromGitHub,
   lsp-plugins,
   bankstown-lv2,
diff --git a/apple-silicon-support/packages/linux-asahi/default.nix b/apple-silicon-support/packages/linux-asahi/default.nix
index 19edef0..cd4b868 100644
--- a/apple-silicon-support/packages/linux-asahi/default.nix
+++ b/apple-silicon-support/packages/linux-asahi/default.nix
@@ -75,12 +75,11 @@ let
       stdenv,
       lib,
       fetchFromGitHub,
-      fetchpatch,
       linuxKernel,
       rustc,
       rust-bindgen,
       ...
-    }@args:
+    }:
     let
       origConfigText = builtins.readFile origConfigfile;
 
@@ -118,8 +117,6 @@ let
         builtins.listToAttrs (map makePair (lib.lists.reverseList configList));
 
       # used to fix issues when nixpkgs gets ahead of the kernel
-      rustAtLeast = version: withRust && (lib.versionAtLeast rustc.version version);
-      bindgenAtLeast = version: withRust && (lib.versionAtLeast rust-bindgen.unwrapped.version version);
     in
     linuxKernel.manualConfig rec {
       inherit stdenv lib;
diff --git a/apple-silicon-support/packages/m1n1/default.nix b/apple-silicon-support/packages/m1n1/default.nix
index 7628ed5..d117d5a 100644
--- a/apple-silicon-support/packages/m1n1/default.nix
+++ b/apple-silicon-support/packages/m1n1/default.nix
@@ -29,7 +29,7 @@ let
     stdenv = lib.recursiveUpdate buildPackages.stdenv stdenvOpts;
   };
   rustPackages = rust.packages.stable.overrideScope (
-    f: p: {
+    _f: p: {
       rustc-unwrapped = p.rustc-unwrapped.override {
         stdenv = lib.recursiveUpdate p.rustc-unwrapped.stdenv stdenvOpts;
       };
diff --git a/iso-configuration/default.nix b/iso-configuration/default.nix
index 1ae8e42..9397e01 100644
--- a/iso-configuration/default.nix
+++ b/iso-configuration/default.nix
@@ -1,7 +1,6 @@
 # configuration that is specific to the ISO
 {
   config,
-  pkgs,
   lib,
   ...
 }:
diff --git a/iso-configuration/installer-configuration.nix b/iso-configuration/installer-configuration.nix
index 4970784..129de68 100644
--- a/iso-configuration/installer-configuration.nix
+++ b/iso-configuration/installer-configuration.nix
@@ -98,13 +98,13 @@
   };
 
   nixpkgs.overlays = [
-    (final: prev: {
+    (_final: prev: {
       # disabling pcsclite avoids the need to cross-compile gobject
       # introspection stuff which works now but is slow and unnecessary
       libfido2 = prev.libfido2.override {
         withPcsclite = false;
       };
-      openssh = prev.openssh.overrideAttrs (old: {
+      openssh = prev.openssh.overrideAttrs (_old: {
         # we have to cross compile openssh ourselves for whatever reason
         # but the tests take quite a long time to run
         doCheck = false;

@ghost

ghost commented Jul 4, 2025

Copy link
Copy Markdown
Author

I can optionally add deadnix as a formatter.

@ghost ghost mentioned this pull request Jul 4, 2025
@ghost

ghost commented Jul 15, 2025

Copy link
Copy Markdown
Author

I've added deadnix as a formatter, this has the effect of introducing a '_' character to functions whose parameters are unused as well as cleaning up any unused code.

I've also reworded some commits and updated the ignore revision.

@ghost ghost mentioned this pull request Jul 18, 2025
13 tasks
@tpwrules

tpwrules commented Jul 20, 2025

Copy link
Copy Markdown
Collaborator

My personal thoughts:

  • It seems reasonable to format the code and I'd be okay with keeping it consistent with NixOS organization decisions. I will withhold complaints about their style choices :)
  • I would even be okay with running it automatically in CI.
  • Can we do this without an extra flake input? Is the formatter not already in nixpkgs? Is the code to hook it up worth the extra input?
  • The extra flake brings in an extra nixpkgs, would much prefer to override to use the same one as the main flake.
  • I don't like the dead code removal. It's not smart enough to remove the associated comments and it's just going to introduce churn on things that are used or not depending on exact project state.

@ghost

ghost commented Jul 20, 2025

Copy link
Copy Markdown
Author

Responding to @tpwrules

  • Any formatter can be chosen, since the issue at hand is that without a formatter, formatting related merge conflicts become an issue. Since nixfmt-rfc-style is now being standardized into nixfmt[1] and we are under nix-community this seems like the best choice.
  • A simple lint workflow in PRs would be fine, nothing too fancy.
  • If we do this without the additional flake input, then we'd also have to track a treefmt.toml and do the glue code ourselves. I just set it so that treefmt-nix follows this flake's nixpkgs. If having the treefmt-nix input removed is more desirable, I can rework the code to do so.
  • Removed deadnix as a default formatter.

1: https://github.com/NixOS/nixfmt/releases/tag/v1.0.0

@ghost

ghost commented Jul 20, 2025

Copy link
Copy Markdown
Author

Resolved merge conflicts and rebased on current main.

Jasi added 3 commits July 26, 2025 15:16
Enables formatting support via `nix fmt` and `nix flake check` for Nix
code.
@ghost

ghost commented Jul 26, 2025

Copy link
Copy Markdown
Author

Resolved merge conflicts and rebased on main.

@flokli

flokli commented Jul 30, 2025

Copy link
Copy Markdown
Member

@normalcea I'm ok with nixfmt-rfc-style, it seems everyone converges to it.

Let's add CI soon, to prevent something accidentially being merged unformatted, but that can be a followup. Thanks!

@tpwrules tpwrules left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested that nix fmt works and makes the same set of changes. Also checked that it doesn't change the system derivation.

@tpwrules tpwrules merged commit 9c936c2 into nix-community:main Aug 2, 2025
@ghost ghost deleted the formatting branch August 2, 2025 17:02
@tpwrules

tpwrules commented Aug 3, 2025

Copy link
Copy Markdown
Collaborator

@normalcea

I upgraded nixpkgs to a two month newer revision and the new nix fmt makes a 180 line diff. Upstream nixfmt just did a v1.0.0 release and they say "just think of it as an entirely new formatting". This is a disappointing outcome especially so soon.

Can you help me understand why the difference is so big? I would have expected the nixfmt-rfc-style you say you used would be approximately the same format. Surely nixpkgs is not about to redo all of its formatting effort?

Maybe the wrong version got picked when I requested that we follow this flake's nixpkgs?

@tpwrules

tpwrules commented Aug 3, 2025

Copy link
Copy Markdown
Collaborator

It looks like there was actually that much difference between nixfmt-rfc-style unstable-2025-04-04 and nixfmt v1.0.0. Nixpkgs just did another massive reformatting commit to adapt as part of NixOS/nixpkgs#427437 .

Are other users just doing the same?

@ghost

ghost commented Aug 3, 2025

Copy link
Copy Markdown
Author

Yes there should be another treewide re-format similar to this PR when the flake inputs are upgraded.

@tpwrules

tpwrules commented Aug 3, 2025

Copy link
Copy Markdown
Collaborator

Okay, I didn't realize that was a necessary step. We'll see how that goes. I guess it's good that we can control exactly when the formatter changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants